Catalog dashboard page updates #760
Conversation
| ) : ( | ||
| <div | ||
| className={styles.root} | ||
| onClick={() => onSelect(subject, courseNumber, number)} |
There was a problem hiding this comment.
Instead of using an onClick event, you can just use a link:
<Link to={`/catalog/${year}/${semester}/${subject}/${courseNumber}/${number}`}>
...
</Link>You could also instead use a ClassDialog for minimal impact on navigation:
<ClassDialog
year={year}
semester={semester}
subject={subject}
courseNumber={courseNumber}
number={number}
>
...
</ClassDialog>There was a problem hiding this comment.
I couldn't find a ClassDialog component, so using Link instead
| </p> | ||
| {term.startDate && term.endDate ? ( | ||
| <p className={styles.paragraph}> | ||
| {formatDate(new Date(term.startDate))} through{" "} |
There was a problem hiding this comment.
Instead of using a custom formatDate function, I've already installed and imported Moment.js elsewhere that you can use:
{moment(term.startDate).format("MMMM Do")}
| </Carousel> | ||
| {recents.length == 0 ? ( | ||
| <></> |
There was a problem hiding this comment.
You can remove unnecessary fragments by using a logical AND:
{recents.length > 0 && (...)}
| @@ -232,6 +234,14 @@ export default function Class({ | |||
| // TODO: Error state | |||
| if (!course || !_class || !pin) { | |||
| return <></>; | |||
| } else { | |||
| addToRecent({ | |||
There was a problem hiding this comment.
Right now, addToRecent will be called on every render. You should instead use a useEffect for this:
useEffect(() => {
if (!course || !_class) return;
addToRecent({
subject: _class.subject,
year: _class.year,
semester: _class.semester,
courseNumber: _class.courseNumber,
number: _class.number,
});
}, [course, _class]);
| @@ -254,7 +264,7 @@ export default function Class({ | |||
| [styles.active]: bookmarked, | |||
| })} | |||
| onClick={() => bookmark()} | |||
| disabled={userLoading} | |||
| disabled={userLoading} // setting disabled to false still appears on my version | |||
There was a problem hiding this comment.
This is a bug I will address!
There was a problem hiding this comment.
There's a similar bug with the switch term select on the Dashboard component. It sets data-disabed="false" which disables the select
apps/frontend/src/lib/recents.ts
Outdated
| newRcd.number == rcd.number), | ||
| false | ||
| ); | ||
| if (alreadyExists) return; |
There was a problem hiding this comment.
It might be nice to move the class to the back of the array if it already exists and limit the recently viewed list to n number of classes.
Need help checking two things:
disable={false}causes the disabled attribute to still be there on my local machine (with value "false"), when it should be removed.Will integrate "View all" bookmark button with profile once both these branches are merged